Convert verification request and transaction to protocols #1528
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of an ongoing work to implement Rust-based device verification I need to prepare the ground by converting existing
MXKeyVerificationRequest
,MXKeyVerificationTransaction
andMXSASTransaction
classes into protocols, so that future PRs can implement these protocols withMatrixCryptoSDK
objects. This is not a full list of classes that need to be hidden behind an interface, only those directly needed for SAS flow.I have previously taken a subclassing approach to solve a similar problem, but this was because refactoring existing code for
MXCrypto
class was too risky without extensive test coverage and it was best to leave this untouched. Swapping request and transaction objects for protocols should be safer, and now validated by some newly enabled integration tests. Subclassing existing implementation on the other hand leaves more technical debt behind and does not safeguard against some methods not being overriden.Regarding naming, I considered both alternatives where protocol adopts existing name (e.g.
MXKeyVerificationRequest
) and subclasses extend this (e.g. Default, V2 ...), as well as naming the protocolMXKeyVerificationRequestProtocol
and leaving the implementation names intact. Reviewing the SDK I did not find many examples of addingProtocol
to protocol definitions so I opted for the first naming variant to preserve consistency. This choice also means that only objective-c files need to perform renames, whereas swift files remain unchanged.